Skip to content

Feature/add bom actions#6

Open
idelcano wants to merge 20 commits intomasterfrom
feature/add_bom_actions
Open

Feature/add bom actions#6
idelcano wants to merge 20 commits intomasterfrom
feature/add_bom_actions

Conversation

@idelcano
Copy link
Collaborator

@idelcano idelcano commented Mar 11, 2026

📌 References

Tests:
https://github.com/EyeSeeTea/github-workflows/tree/feature/test_bom_actions
https://github.com/EyeSeeTea/user-extended-app/actions/runs/22951772608/workflow?pr=337
https://github.com/EyeSeeTea/user-extended-app/actions/runs/22951772608/job/66617978632?pr=337

📝 Implementation

This PR introduces the Dependency-Track GitHub Actions workflows to support SBOM generation, upload, and security reporting integration.

It includes:

New reusable workflows for Dependency-Track (yarn4 and syft variants)

Supporting scripts for BOM upload, metrics retrieval, SARIF normalization, and code scanning reporting
I have test this branch with some small changes to point to the test branch here, but the idea is make it work on master/development:
https://github.com/EyeSeeTea/github-workflows/tree/feature/test_bom_actions

For now we are not going to change the self-hosted setup, at least in this branch. However, some errors might appear when the old runners end up using the new runner, which is called dependency-track-runner.

A test run has been created for validation in user-extended-app:
https://github.com/EyeSeeTea/user-extended-app/actions/runs/22951772608/workflow?pr=337

I expect to create a new branch pointing to the correct GitHub branch once this workflow is approved in the "client" repositories.
changing:

uses: EyeSeeTea/github-workflows/.github/workflows/dependency-track-syft.yml@feature/test_bom_actions

uses: EyeSeeTea/github-workflows/.github/workflows/dependency-track-syft.yml@master

: Purpose

Detect dependency vulnerabilities early during PR validation
Publish findings to GitHub Code Scanning (Security tab) so developers can review and triage them
Improve visibility of newly introduced dependency risk during development

Why two workflows

yarn4 method: generally more reliable in our setup for SBOM generation and finding mapping
syft method: less reliable today, but more representative for older build contexts/projects that do not use Yarn 4
Developer experience

When dependency issues are found, the GitHub Action provides:
A direct link to the detected findings in github security
A summary of issues grouped by severity
Best-effort detection of newly introduced vulnerabilities

📹 Screenshots/Screen capture

🔥 Testing

@idelcano idelcano requested a review from gqcorneby March 11, 2026 12:28
Copy link
Contributor

@gqcorneby gqcorneby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @idelcano! I just have a few comments and clarifications :) I also tested calling the master workflow from this feature branch here:
https://github.com/EyeSeeTea/dhis2-app-skeleton/pull/103/changes

There are a few errors on the scripts it's trying to run.
bash: shared-workflows/scripts/upload_bom_and_fetch_metrics.sh: No such file or directory

edit: ignore above. The error is because it's trying to fetch the scripts from master branch

yarn install

# Install syft in the container
curl -sSfL https://raw.githubusercontent.com/anchore/syft/main/install.sh \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This installation pipes the main install script directly from a remote source. I'm not fully familiar with the specific risks here, but executing a mutable remote script could potentially introduce supply-chain risks if the script changes. For CI workflows, it might be worth considering downloading a specific Syft release binary instead to make the build more deterministic and reduce potential risks.

https://github.com/anchore/syft/releases

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally.
It's better to use a versioned URL for the install script or download a specific release to ensure the process is deterministic and secure. I will change it.

if: github.event_name == 'pull_request' && (contains(inputs.workflows_to_run, 'dependency-track-yarn4') || contains(inputs.workflows_to_run, 'all'))
uses: ./.github/workflows/dependency-track-yarn4.yml
secrets:
DTRACK_API_KEY: ${{ secrets.DTRACK_API_KEY }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DTRACK_API_KEY is optional in the master workflow but required in the dependency-track workflows. Since workflows_to_run defaults to "all", the dependency-track jobs may be triggered even when the secret is not provided.

Should we add a guard in the if condition to avoid invoking those workflows without the required secret?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The secret is set at the organization level, so I assume it will always be there. If it were missing, it would trigger an error and we'd notice it's failing. We could add the 'if' condition, but wouldn't that just add unnecessary complexity?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aaaaah okay okay. The option in the master file will just act as an override if ever. Got it! You're right, no need to add the if condition

DTRACK_API_KEY:
required: true
workflow_dispatch:
pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to confirm if we need this trigger. This would refer to pull requests on this repository itself, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The master repository is not required to run its own GitHub Actions on PRs for now.
The idea is for the 'child' repositories to trigger these workflows whenever a developer opens or updates a PR.
I added pull_request to the master workflow while testing to make it work in some moment, but maybe it's not required.
Now that it's verified, we might be able to remove it; I'll give that a try.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally i have removed it, looks like work right setting it in child repo! Thank you!

uses: ./.github/workflows/app-test.yml
with:
runner: ${{ inputs.runner || (github.event.repository.private && 'self-hosted') || 'ubuntu-latest' }}
runner: ${{ inputs.runner || (github.event.repository.private && 'self-runner') || 'ubuntu-latest' }}
Copy link
Contributor

@gqcorneby gqcorneby Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirming this. You mentioned:

For now we are not going to change the self-hosted setup

But it looks like self-hosted was updated to self-runner. Is self-runner the new tag for the default self-hosted runner?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting that. It's my mistake; I changed the tag initially and didn't restore it correctly.

@idelcano idelcano requested a review from gqcorneby March 13, 2026 08:22
Copy link
Contributor

@gqcorneby gqcorneby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @idelcano!

@adrianq adrianq requested a review from tokland March 16, 2026 15:13
Copy link

@tokland tokland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in EyeSeeTea/user-extended-app#338 I get: Error: New critical/high alert instances detected vs base (critical=9, high=31, medium=33, low=2)

Which I guess it's the expected output.

And is this normal? https://github.com/EyeSeeTea/user-extended-app/actions/runs/23236041522/job/67540090242?pr=338#step:6:34

Some in-line comments about the PR implementation:

@@ -0,0 +1,709 @@
#!/usr/bin/env python3
Copy link

@tokland tokland Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The script looks reasonably good (does it come from some project or AI generated?). However, it's quite a bunch of code, so the only realistic way to assess if it's doing what's expected is adding some tests using real files as input.

I am aware we don't have a lot of extra time for this task, so maybe that won't be possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I used GPT-5.4-Codex to build it, testing and iterating as I went. It took some time to get everything integrated correctly.

I agree that starting with tests would have been better—I'll keep that in mind for the future, as it would have saved us all some time. It started as a single YAML, but as it grew, and when everything was working I moved the logic to separate files.

As Nacho suggested, I’ve added some TODOs so we can add those formal tests later when we have more time.

@adrianq
Copy link
Member

adrianq commented Mar 18, 2026

@idelcano We have merged some code into master. On top of @tokland 's comment, if you can merge your branch with master it would be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants